-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore: yarn upgrade dependencies requiring intervention #36600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: yarn upgrade dependencies requiring intervention #36600
Conversation
03d3d8e to
f938a7a
Compare
|
|
||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Merge Queue Status🚫 The pull request has left the queue (rule: This pull request spent 3 hours 50 minutes 28 seconds in the queue, with no time running CI. Required conditions to merge
ReasonThe pull request #36600 has been manually updated HintIf you want to requeue this pull request, you can post a |
Ran npm-check-updates and yarn upgrade to keep the `yarn.lock` file up-to-date.
c129ed2 to
5af01f7
Compare
…36690) ### Issue # (if applicable) Fixes false positive failures in Security Guardian for PR #36600 ### Reason for this change The `guardhooks-no-root-principals-except-kms-secrets.guard` rule was incorrectly failing on CloudFormation templates containing unresolved intrinsic functions (like `Fn::Join`, `Fn::Sub`) in IAM policy principals. This caused false positive security violations on legitimate CDK-generated templates. **Error encountered:** ``` Error = [PathAwareValues are not comparable map, Regex] Value = {"Fn::Join":["",["arn:",{"Ref":"AWS::Partition"},":iam::234567890123:role/..."]]} ComparedWith = "/(?i):root$/" ``` ### Description of changes **Root Cause:** The rule had an inconsistency where: - `AssumeRolePolicyDocument` section properly checked if array items were strings before validation - `PolicyDocument`, `Policy`, and `ResourcePolicy` sections were **missing** this type check This caused cfn-guard to attempt regex comparison on intrinsic function objects, resulting in comparison errors. **Fix Applied:** Changed the validation pattern from: ```guard when Principal.AWS is_list { Principal.AWS[*] != /(?i):root$/ # Fails on objects } ``` To: ```guard when Principal.AWS is_list { Principal.AWS[*] { when this is_string { this != /(?i):root$/ # Only validates strings } } } ``` This pattern: 1. Iterates through each array item individually 2. Only validates items that are strings 3. Skips intrinsic function objects (avoiding false positives on static templates) 4. Still catches real `:root` principals in resolved templates **Files Changed:** - `tools/@aws-cdk/security-guardian/rules/guard-hooks/guardhooks-no-root-principals-except-kms-secrets.guard` **Alternatives Considered:** - Adding `when Principal.AWS[*] is_string` guard: This checks if ALL items are strings, which fails when even one intrinsic function is present - Disabling the rule for static templates: Would miss real violations in templates without intrinsics - The chosen solution properly handles mixed arrays and maintains security validation ### Describe any new or updated permissions being added No IAM permissions changes. ### Description of how you validated changes Unit tests ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Merge Queue Status✅ The pull request has been merged at 38ea102 This pull request spent 28 minutes 57 seconds in the queue, including 28 minutes 46 seconds running CI. Required conditions to merge
|
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Ran npm-check-updates and yarn upgrade for the following dependencies:
Checkout this branch and run integration tests locally to update snapshots.
See https://www.npmjs.com/package/@aws-cdk/integ-runner for more integ runner options.